-
Notifications
You must be signed in to change notification settings - Fork 52
feat(urunc-deploy): install artifacts under /opt/urunc #387
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
✅ Deploy Preview for urunc ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
4b61cd6 to
ce21fa6
Compare
|
Hi @cmainas , can u please take a look and let me know if any more changes are required. Thanks! |
cmainas
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hello @spline2hg ,
thank you for this PR. Overall, it looks good, but there are a few points that we could improve:
- Could you also update this subsection in the documentation which is related to
urunc-deploy? - I think there is no reason for the install script to "generate" the config file, since most of the values are static. We can change that in the future, where we allow specific configuration options during deployment to pass to the configuration. However, for the time being (and as first iteration) we can simply use a static file that the script installs like any other artifact.
cmainas
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hello @spline2hg ,
please do not perform changes that are not related to the respective PR/issue.
Also, I do not understand why urunc should be updated to search /opt/urunc; it should only check /usr/share. The monitor artifacts and files are not urunc specific ones. They can be system wide (e.g. installation through a package manager). This adds unnecessary overhead and users are already able to specify any directory through the config.
|
Yes @cmainas, sorry about that, I got a bit confused there. I have removed the unrelated changes. |
cmainas
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hello @spline2hg ,
I have added two small comments that need to get address. Other than that please make sure to:
- Rebase your changes with the main branch of urunc
- Squash your commits in one
- Add yourself to https://github.com/spline2hg/urunc/blob/urunc-deploy-config/.github/contributors.yaml
so we can merge this PR.
|
Also, make sure to fix your commit message. More information at https://urunc.io/developer-guide/contribute/ |
011f2f2 to
9ab654e
Compare
|
@cmainas made the changes. |
|
Hello @spline2hg , please make sure to resolve the commit linter issues Also, another rebase will be necessary. |
Previously, urunc-deploy installed hypervisor binaries to /usr/local/bin and QEMU data files to /usr/local/share. This commit reorganizes the installation to use /opt/urunc as the base directory for all urunc artifacts. A config.toml file is now installed to /etc/urunc to instruct urunc about the new binary locations. Signed-off-by: spline2hg <181270613+spline2hg@users.noreply.github.com>
9ab654e to
df1a190
Compare
|
@cmainas fixed the commit linter issue and rebased. |
cmainas
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you @spline2hg for the changes.
Previously, urunc-deploy installed hypervisor binaries to /usr/local/bin and QEMU data files to /usr/local/share. This commit reorganizes the installation to use /opt/urunc as the base directory for all urunc artifacts. A config.toml file is now installed to /etc/urunc to instruct urunc about the new binary locations. PR: #387 Signed-off-by: spline2hg <181270613+spline2hg@users.noreply.github.com> Reviewed-by: Charalampos Mainas <cmainas@nubificus.co.uk> Approved-by: Charalampos Mainas <cmainas@nubificus.co.uk>
Previously, urunc-deploy installed hypervisor binaries to /usr/local/bin and QEMU data files to /usr/local/share. This commit reorganizes the installation to use /opt/urunc as the base directory for all urunc artifacts. A config.toml file is now installed to /etc/urunc to instruct urunc about the new binary locations. PR: #387 Signed-off-by: spline2hg <181270613+spline2hg@users.noreply.github.com> Reviewed-by: Charalampos Mainas <cmainas@nubificus.co.uk> Approved-by: Charalampos Mainas <cmainas@nubificus.co.uk>
Summary
Fixes #379
Changes urunc-deploy to install all artifacts under
/opt/uruncdirectory instead of scattering them across multiple directories. Also auto-generates/etc/urunc/config.tomlwith the correct paths.